feat: Derive fiat asset from feature flags before hardcoded fallback#8631
feat: Derive fiat asset from feature flags before hardcoded fallback#8631
Conversation
OGPoyraz
had a problem deploying
to
default-branch
GitHub Actions
Failure
OGPoyraz
force-pushed
the
ogp/fiat-ff-implementation
branch
from
5425439 to
b24eac7
Compare
OGPoyraz
changed the title
OGPoyraz
force-pushed
the
ogp/fiat-ff-implementation
branch
2 times, most recently
from
7ef80a6 to
1096df7
Compare
| import type { TransactionPayFiatAsset } from './constants'; | ||
| import { ETH_MAINNET_FIAT_ASSET, FIAT_ASSET_ID_BY_TX_TYPE } from './constants'; | ||
|
|
||
| function resolveTransactionType( |
There was a problem hiding this comment.
Minor, exported functions at top of file for readability?
| transaction: TransactionMeta, | ||
| ): TransactionType | undefined { | ||
| if (transaction.type === TransactionType.batch) { | ||
| return transaction.nestedTransactions?.[0]?.type; |
There was a problem hiding this comment.
Is it safe to assume this is always the first call?
Don't we iterate over calls looking for a supported type in Relay strategy?
In Predict for example, if first deposit, the initial call is the deploy for Safe proxy?
There was a problem hiding this comment.
Done, added a similar utility
| const firstMatchingType = transaction.nestedTransactions?.[0]?.type; | ||
| if (firstMatchingType) { | ||
| return FIAT_ASSET_ID_BY_TX_TYPE[firstMatchingType]; | ||
| const hardcodedAsset = FIAT_ASSET_ID_BY_TX_TYPE[txType]; |
There was a problem hiding this comment.
The intent of the feature-flag utils is to abstract these defaults also, so most of the existing functions fallback to defaults.
Can we do that here and encapsulate the constant and mainnet fallbacks in there?
There was a problem hiding this comment.
Good call, carried over the fallbacks info FF utility
| transactionType: TransactionType, | ||
| ): TransactionPayFiatAsset | undefined { | ||
| const state = messenger.call('RemoteFeatureFlagController:getState'); | ||
| const fiatFlags = state.remoteFeatureFlags?.confirmations_pay_fiat as |
There was a problem hiding this comment.
Should we define a local FiatFlags type at the top of the file for re-use and readability?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c647d8a23669a02f76637ef87693291937c1505. Configure here.
OGPoyraz
force-pushed
the
ogp/fiat-ff-implementation
branch
from
0c647d8 to
469e1c7
Compare
2 participants
Explanation
Currently
deriveFiatAssetForFiatPaymentresolves the fiat asset for a payment entirely from a hardcoded map (FIAT_ASSET_ID_BY_TX_TYPE). This makes it impossible to adjust asset mappings without a code release.This PR introduces a 3-tier resolution for fiat assets:
confirmations_pay_fiat.assetPerTransactionType[txType]viaRemoteFeatureFlagControllerFIAT_ASSET_ID_BY_TX_TYPEconstantThe function signature gains a
messengerparameter (already available at the call site infiat-quotes.ts), and the return type tightens fromTransactionPayFiatAsset | undefinedtoTransactionPayFiatAssetsince the ETH mainnet fallback guarantees a value.References
confirmations_pay_fiatChecklist
Note
Medium Risk
Changes how fiat strategy chooses/identifies its source asset by introducing remote-flag overrides and new CAIP-19 asset ID construction, which could affect quoting/submission if mappings or chain coin types are incorrect.
Overview
Fiat asset selection for MM Pay is refactored to a 3-tier resolution:
confirmations_pay_fiat.assetPerTransactionTyperemote feature flag, then the existing hardcodedFIAT_ASSET_ID_BY_TX_TYPE, then an ETH mainnet fallback (including improved handling of batch transactions by scanning nested tx types).Fiat quote/submit flows and
TransactionPayControllertoken selection now derive theRampsControllerassetIdvia a newbuildCaipAssetType(chainId, address)helper (SLIP-44 for native tokens with chain overrides, ERC-20 otherwise) and pull token decimals fromgetTokenInfoinstead of embedding them in fiat-asset constants; tests/changelog are updated accordingly.Reviewed by Cursor Bugbot for commit c66a60a. Bugbot is set up for automated code reviews on this repo. Configure here.